Dev include bug #2
Conversation
- Next.js 14 (App Router) + TypeScript + Prisma + Tailwind CSSで実装 - TODOの追加、一覧表示、完了/未完了の切り替え、削除機能を実装 - REST APIエンドポイント（GET, POST, PATCH, DELETE）を実装 - コンポーネントテストとAPIテストを実装 - SQLiteデータベースを使用（PostgreSQL等への移行が容易） - Zodによる入力検証を実装
📝 WalkthroughWalkthroughThis pull request establishes a complete Next.js-based TODO management application with a full tech stack. It introduces API routes for CRUD operations, React components for the UI (form and todo list), database schema with Prisma, styling with Tailwind CSS, comprehensive test coverage, and all necessary configuration files for development and deployment. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @.gitignore:
- Around line 27-29: The repo ignores .env and .env*.local (see .env*.local and
.env in the diff) but lacks a template for required variables; add a new file
named .env.example (or .env.template) to the repository containing all required
environment variable names with safe placeholder values and brief inline
comments describing each one, ensure the example is committed (not ignored) and
kept in sync with any changes to how the app reads env vars (e.g., variables
referenced in startup code or config).
In `@README.md`:
- Around line 33-35: Update the fenced code block around the DATABASE_URL
example to include a language specifier (e.g., use "dotenv" or "text") so the
linter and syntax highlighter recognize it; locate the triple-backtick block
containing DATABASE_URL="file:./prisma/dev.db" and change the opening fence to
```dotenv (or ```text) while leaving the content unchanged.
- Around line 66-83: Update the fenced code block in README.md that shows the
project tree (starts with the line "todo-review-test/" and the tree lines like
"├── app/") to include a language specifier by changing the opening backticks
from ``` to ```text so the directory structure is rendered correctly as plain
text.
In `@tests/components/TodoForm.test.tsx`:
- Around line 41-82: Add an assertion that window.location.reload was invoked
after the successful POST in the TodoForm test: after the waitFor that checks
mockFetch, add expect(reloadSpy).toHaveBeenCalled() (or
.toHaveBeenCalledTimes(1)) to verify reload was triggered, then restore the spy
with reloadSpy.mockRestore(); reference the existing reloadSpy,
window.location.reload, mockFetch, and TodoForm to locate where to insert this
assertion.
In `@tests/components/TodoItem.test.tsx`:
- Around line 43-74: Add an assertion that the onUpdate callback is invoked
after the successful PATCH response: after the existing fetch assertion, await a
waitFor (or use expect with async) to assert mockOnUpdate was called with the
updated todo object (e.g., the original mockTodo with completed: true);
reference the TodoItem component props and the mockOnUpdate and mockFetch mocks
to locate where to add this check.
- Around line 76-101: The test for deletion in TodoItem is missing an assertion
that the onDelete callback (mockOnDelete) is invoked after a successful DELETE;
update the test to assert mockOnDelete was called (e.g.,
expect(mockOnDelete).toHaveBeenCalled() or toHaveBeenCalledWith(mockTodo.id))
after the mockFetch resolves — place this assertion after or inside the existing
waitFor that checks fetch so it runs once the deletion completes.
🧹 Nitpick comments (6)
components/TodoForm.tsx (1)
36-38: Consider avoiding full page reload.Using
window.location.reload()triggers a full page reload, which is inefficient and provides poor UX. Consider using a callback prop to notify the parent component, or use Next.jsuseRouter().refresh()for a smoother experience.♻️ Suggested approach using callback prop
-export default function TodoForm() { +export default function TodoForm({ onSuccess }: { onSuccess?: () => void }) {Then in the success handler:
setTitle('') setDescription('') - // ページをリロードしてTODOリストを更新 - window.location.reload() + // 親コンポーネントに成功を通知 + onSuccess?.()app/api/todos/route.ts (1)
28-55: Consider handling malformed JSON separately.If
request.json()fails due to malformed JSON, it will be caught by the outer catch and return a 500 error. Returning a 400 Bad Request would be more appropriate for invalid input.Proposed improvement
export async function POST(request: NextRequest) { try { - const body = await request.json() + let body + try { + body = await request.json() + } catch { + return NextResponse.json( + { error: 'リクエストボディが不正です' }, + { status: 400 } + ) + } const validatedData = todoSchema.parse(body)app/api/todos/[id]/route.ts (3)
62-67: Fragile error detection relying on message string matching.Checking
error.message.includes('Record to update not found')is brittle — Prisma error messages can change between versions or locales. Consider using Prisma's error codes instead.♻️ Suggested improvement using Prisma error codes
+import { Prisma } from '@prisma/client' + // In PATCH handler: - if (error instanceof Error && error.message.includes('Record to update not found')) { + if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === 'P2025') { return NextResponse.json( { error: 'TODOが見つかりません' }, { status: 404 } ) }
89-94: Same fragile error detection in DELETE handler.Apply the same Prisma error code check here for consistency and robustness.
♻️ Suggested improvement
- if (error instanceof Error && error.message.includes('Record to delete does not exist')) { + if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === 'P2025') { return NextResponse.json( { error: 'TODOが見つかりません' }, { status: 404 } ) }
44-54: Consider validating the ID format before database operations.If IDs follow a specific format (e.g., UUID or CUID), validating before the Prisma call provides clearer error messages and avoids unnecessary database round-trips.
components/TodoList.tsx (1)
43-49: Consider optimistic updates for better UX.Currently, both
handleTodoUpdateandhandleTodoDeleterefetch the entire list. For a snappier experience, you could apply optimistic updates (update local state immediately, then reconcile if the API call fails). This is optional but improves perceived performance.
📜 Review details
Configuration used: Environment YAML
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonprisma/prisma/dev.dbis excluded by!**/*.db
📒 Files selected for processing (23)
.eslintrc.json.gitignoreREADME.mdapp/api/todos/[id]/route.tsapp/api/todos/route.tsapp/globals.cssapp/layout.tsxapp/page.tsxcomponents/TodoForm.tsxcomponents/TodoItem.tsxcomponents/TodoList.tsxlib/prisma.tsnext.config.jspackage.jsonpostcss.config.jsprisma/schema.prismatailwind.config.tstests/api/todos.test.tstests/components/TodoForm.test.tsxtests/components/TodoItem.test.tsxtests/setup.tstsconfig.jsonvitest.config.cjs
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md
33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
66-66: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (37)
.gitignore (1)
1-40: LGTM! Well-structured .gitignore for Next.js + Prisma.The file correctly covers all necessary ignore patterns for the tech stack, including dependencies, build artifacts, environment files, and Prisma database files. The patterns are appropriate and follow standard best practices.
.eslintrc.json (1)
1-3: LGTM!Standard Next.js ESLint configuration extending the recommended
next/core-web-vitalspreset.tailwind.config.ts (1)
1-19: LGTM!Well-structured Tailwind configuration with proper content paths for Next.js App Router and theme extension using CSS variables that align with the definitions in
app/globals.css.app/globals.css (1)
1-27: LGTM!Clean global CSS setup with Tailwind directives, well-defined CSS variables for light/dark theme support, and a useful
text-balanceutility class. The CSS variables properly correspond to the theme extension intailwind.config.ts.postcss.config.js (1)
1-6: LGTM!Standard PostCSS configuration for Next.js with Tailwind CSS. The plugin order is correct with
tailwindcssprocessing beforeautoprefixer.lib/prisma.ts (1)
1-13: LGTM! Standard Prisma singleton pattern for Next.js.This correctly prevents multiple PrismaClient instances during hot module reloading in development, and the environment-aware logging configuration is appropriate.
tsconfig.json (1)
1-27: LGTM! Standard Next.js 14 TypeScript configuration.The configuration follows Next.js conventions with appropriate strict mode, bundler module resolution, and path aliasing.
prisma/schema.prisma (1)
1-23: LGTM! Well-structured Prisma schema for the TODO model.Good choices include:
cuid()for URL-safe, collision-resistant IDs- Index on
completedfor efficient filtering queries@updatedAtfor automatic timestamp managementpackage.json (2)
5-18: Well-organized scripts configuration.The scripts cover development workflow comprehensively:
- Build correctly chains
prisma generatebeforenext build- Database commands (
db:push,db:generate,db:studio,db:migrate) provide full Prisma workflow- Test commands support run, watch, and coverage modes
42-42: No issues found. Vitest version 4.0.14 is valid and current.app/layout.tsx (1)
1-19: LGTM! Clean and minimal root layout.The layout correctly:
- Sets
lang="ja"matching the Japanese metadata- Uses proper TypeScript typing with
Readonly<{ children: React.ReactNode }>- Imports global styles appropriately
next.config.js (1)
1-6: LGTM!Clean minimal Next.js configuration with
reactStrictModeenabled, which is a good practice for catching potential issues during development.components/TodoForm.tsx (2)
1-9: LGTM!State management is well-structured with appropriate separation of concerns for form inputs, submission state, and error handling.
46-90: LGTM!Well-structured form with proper accessibility attributes (labels with
htmlFor), input validation, and appropriate disabled states during submission. The error display and button state management are correctly implemented.tests/setup.ts (1)
1-1: LGTM!Standard setup for extending Vitest with jest-dom matchers, enabling DOM-specific assertions in component tests.
app/page.tsx (1)
1-20: LGTM!Clean page composition with proper server/client component boundary. The layout structure with centered card and responsive styling is well-implemented.
tests/api/todos.test.ts (3)
1-13: LGTM with minor note on mock setup.The Prisma mock structure is appropriate for the tested scenarios. The mock only includes
findManyandcreatemethods, which aligns with the GET and POST test coverage.
20-60: LGTM!Good test coverage for the GET endpoint, including both success and error scenarios. The mock assertions verify status codes and response data appropriately.
62-110: LGTM!Well-structured POST tests covering successful creation and title validation. The Request object construction with proper headers and JSON body is correct.
vitest.config.cjs (1)
1-24: LGTM!The Vitest configuration is well-structured with appropriate settings for a Next.js application:
- Path alias
@correctly maps to the project root- jsdom environment is appropriate for React component testing
- Coverage configuration targets the right directories
tests/components/TodoForm.test.tsx (3)
1-12: LGTM!Test setup is clean with proper imports and mock clearing between tests.
14-39: LGTM!Good coverage of form rendering and button state validation. The tests correctly verify the disabled state logic based on title input.
84-104: LGTM!Error handling test properly verifies that error messages are displayed to the user when the API call fails.
tests/components/TodoItem.test.tsx (2)
1-28: LGTM!Test setup is well-organized with proper mock data and callbacks.
103-116: LGTM!Good test for verifying the visual completed state styling.
app/api/todos/route.ts (2)
1-8: LGTM!Well-defined validation schema with appropriate character limits and localized error messages.
10-26: LGTM!The GET handler is clean and functional. For a TODO application, the lack of pagination is acceptable.
components/TodoItem.tsx (4)
1-10: LGTM!Clean component interface with proper TypeScript typing.
12-43: LGTM!The toggle handler has proper loading state management, error handling, and cleanup in the finally block.
45-69: LGTM!Delete handler with proper confirmation, loading state, and error handling follows a consistent pattern.
71-107: LGTM!Well-structured JSX with proper accessibility (disabled states), conditional styling for completed items, and user feedback for loading/error states.
app/api/todos/[id]/route.ts (2)
5-9: LGTM! Zod schema with appropriate validation constraints.The schema correctly uses
.optional()for partial updates, which is appropriate for a PATCH endpoint. Validation messages are properly localized in Japanese.
12-37: LGTM! GET handler is well-structured.Proper async params handling for Next.js 15, correct 404 handling when todo is not found, and appropriate error logging with generic user-facing error messages.
components/TodoList.tsx (4)
6-13: LGTM! Well-defined interface for Todo type.The interface properly reflects the API response shape with
createdAtandupdatedAtas strings (serialized from Date by JSON).
20-37: LGTM! Solid fetch implementation.Good use of
useCallbackwith empty dependency array, proper error handling, and consistent loading state management in thefinallyblock.
51-71: LGTM! Clear loading and error states with retry capability.The error UI includes a retry button that reuses
fetchTodos, which is appropriate. Accessibility could be enhanced with ARIA attributes on the loading state, but this is minor.
81-119: LGTM! Clean grouping and rendering logic.Filtering into
completedTodosandactiveTodosis straightforward. Each group conditionally renders with a count header. Proper use ofkey={todo.id}for list stability.
| # local env files | ||
| .env*.local | ||
| .env |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if environment template files exist in the repository
# Search for .env.example, .env.template, or similar files
fd -t f '^\.env\.(example|template|sample)$'Repository: admin-coderabbit/todo-review-test
Length of output: 284
🏁 Script executed:
#!/bin/bash
# Description: Check if environment template files exist in the repository (including hidden files)
# Search for .env.example, .env.template, .env.sample, or similar files
fd -H -t f '\.env\.(example|template|sample)'
# Also check for any .env* files to see what exists
echo "=== All .env* files in repository ==="
fd -H -t f '^\.env'Repository: admin-coderabbit/todo-review-test
Length of output: 114
Add a .env.example or .env.template file to the repository.
The repository correctly ignores .env files for security, but developers need a template showing required environment variables. Add a .env.example file with placeholder values for all necessary environment variables.
🤖 Prompt for AI Agents
In @.gitignore around lines 27 - 29, The repo ignores .env and .env*.local (see
.env*.local and .env in the diff) but lacks a template for required variables;
add a new file named .env.example (or .env.template) to the repository
containing all required environment variable names with safe placeholder values
and brief inline comments describing each one, ensure the example is committed
(not ignored) and kept in sync with any changes to how the app reads env vars
(e.g., variables referenced in startup code or config).
| DATABASE_URL="file:./prisma/dev.db" | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to fenced code block.
The static analysis tool flagged this code block as missing a language specifier. Consider adding dotenv or text for syntax highlighting and linter compliance.
📝 Proposed fix
-```
+```dotenv
DATABASE_URL="file:./prisma/dev.db"</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@README.md` around lines 33 - 35, Update the fenced code block around the
DATABASE_URL example to include a language specifier (e.g., use "dotenv" or
"text") so the linter and syntax highlighter recognize it; locate the
triple-backtick block containing DATABASE_URL="file:./prisma/dev.db" and change
the opening fence to ```dotenv (or ```text) while leaving the content unchanged.
| todo-review-test/ | ||
| ├── app/ | ||
| │ ├── api/ | ||
| │ │ └── todos/ # TODO APIエンドポイント | ||
| │ ├── globals.css # グローバルスタイル | ||
| │ ├── layout.tsx # ルートレイアウト | ||
| │ └── page.tsx # ホームページ | ||
| ├── components/ | ||
| │ ├── TodoForm.tsx # TODO追加フォーム | ||
| │ ├── TodoList.tsx # TODO一覧 | ||
| │ └── TodoItem.tsx # TODOアイテム | ||
| ├── lib/ | ||
| │ └── prisma.ts # Prismaクライアント | ||
| ├── prisma/ | ||
| │ └── schema.prisma # データベーススキーマ | ||
| └── tests/ # テストファイル | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to fenced code block.
The project structure code block is missing a language specifier. Use text for directory tree structures.
📝 Proposed fix
-```
+```text
todo-review-test/
├── app/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ``` | |
| todo-review-test/ | |
| ├── app/ | |
| │ ├── api/ | |
| │ │ └── todos/ # TODO APIエンドポイント | |
| │ ├── globals.css # グローバルスタイル | |
| │ ├── layout.tsx # ルートレイアウト | |
| │ └── page.tsx # ホームページ | |
| ├── components/ | |
| │ ├── TodoForm.tsx # TODO追加フォーム | |
| │ ├── TodoList.tsx # TODO一覧 | |
| │ └── TodoItem.tsx # TODOアイテム | |
| ├── lib/ | |
| │ └── prisma.ts # Prismaクライアント | |
| ├── prisma/ | |
| │ └── schema.prisma # データベーススキーマ | |
| └── tests/ # テストファイル | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
66-66: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@README.md` around lines 66 - 83, Update the fenced code block in README.md
that shows the project tree (starts with the line "todo-review-test/" and the
tree lines like "├── app/") to include a language specifier by changing the
opening backticks from ``` to ```text so the directory structure is rendered
correctly as plain text.
| it('TODOの作成が成功する', async () => { | ||
| const user = userEvent.setup() | ||
| const mockFetch = vi.mocked(fetch) | ||
|
|
||
| mockFetch.mockResolvedValueOnce({ | ||
| ok: true, | ||
| json: async () => ({ | ||
| id: '1', | ||
| title: 'テストTODO', | ||
| description: '', | ||
| completed: false, | ||
| createdAt: new Date().toISOString(), | ||
| updatedAt: new Date().toISOString(), | ||
| }), | ||
| } as Response) | ||
|
|
||
| // window.location.reloadをモック | ||
| const reloadSpy = vi.spyOn(window.location, 'reload').mockImplementation(() => {}) | ||
|
|
||
| render(<TodoForm />) | ||
|
|
||
| const titleInput = screen.getByLabelText(/タイトル/) | ||
| const submitButton = screen.getByRole('button', { name: /TODOを追加/ }) | ||
|
|
||
| await user.type(titleInput, 'テストTODO') | ||
| await user.click(submitButton) | ||
|
|
||
| await waitFor(() => { | ||
| expect(mockFetch).toHaveBeenCalledWith('/api/todos', { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| body: JSON.stringify({ | ||
| title: 'テストTODO', | ||
| description: undefined, | ||
| }), | ||
| }) | ||
| }) | ||
|
|
||
| reloadSpy.mockRestore() | ||
| }) |
There was a problem hiding this comment.
Missing assertion for window.location.reload call.
The reloadSpy is set up but never asserted. The test should verify that reload() is called on successful creation.
Proposed fix
await waitFor(() => {
expect(mockFetch).toHaveBeenCalledWith('/api/todos', {
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify({
title: 'テストTODO',
description: undefined,
}),
})
})
+
+ await waitFor(() => {
+ expect(reloadSpy).toHaveBeenCalled()
+ })
reloadSpy.mockRestore()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('TODOの作成が成功する', async () => { | |
| const user = userEvent.setup() | |
| const mockFetch = vi.mocked(fetch) | |
| mockFetch.mockResolvedValueOnce({ | |
| ok: true, | |
| json: async () => ({ | |
| id: '1', | |
| title: 'テストTODO', | |
| description: '', | |
| completed: false, | |
| createdAt: new Date().toISOString(), | |
| updatedAt: new Date().toISOString(), | |
| }), | |
| } as Response) | |
| // window.location.reloadをモック | |
| const reloadSpy = vi.spyOn(window.location, 'reload').mockImplementation(() => {}) | |
| render(<TodoForm />) | |
| const titleInput = screen.getByLabelText(/タイトル/) | |
| const submitButton = screen.getByRole('button', { name: /TODOを追加/ }) | |
| await user.type(titleInput, 'テストTODO') | |
| await user.click(submitButton) | |
| await waitFor(() => { | |
| expect(mockFetch).toHaveBeenCalledWith('/api/todos', { | |
| method: 'POST', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| }, | |
| body: JSON.stringify({ | |
| title: 'テストTODO', | |
| description: undefined, | |
| }), | |
| }) | |
| }) | |
| reloadSpy.mockRestore() | |
| }) | |
| it('TODOの作成が成功する', async () => { | |
| const user = userEvent.setup() | |
| const mockFetch = vi.mocked(fetch) | |
| mockFetch.mockResolvedValueOnce({ | |
| ok: true, | |
| json: async () => ({ | |
| id: '1', | |
| title: 'テストTODO', | |
| description: '', | |
| completed: false, | |
| createdAt: new Date().toISOString(), | |
| updatedAt: new Date().toISOString(), | |
| }), | |
| } as Response) | |
| // window.location.reloadをモック | |
| const reloadSpy = vi.spyOn(window.location, 'reload').mockImplementation(() => {}) | |
| render(<TodoForm />) | |
| const titleInput = screen.getByLabelText(/タイトル/) | |
| const submitButton = screen.getByRole('button', { name: /TODOを追加/ }) | |
| await user.type(titleInput, 'テストTODO') | |
| await user.click(submitButton) | |
| await waitFor(() => { | |
| expect(mockFetch).toHaveBeenCalledWith('/api/todos', { | |
| method: 'POST', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| }, | |
| body: JSON.stringify({ | |
| title: 'テストTODO', | |
| description: undefined, | |
| }), | |
| }) | |
| }) | |
| await waitFor(() => { | |
| expect(reloadSpy).toHaveBeenCalled() | |
| }) | |
| reloadSpy.mockRestore() | |
| }) |
🤖 Prompt for AI Agents
In `@tests/components/TodoForm.test.tsx` around lines 41 - 82, Add an assertion
that window.location.reload was invoked after the successful POST in the
TodoForm test: after the waitFor that checks mockFetch, add
expect(reloadSpy).toHaveBeenCalled() (or .toHaveBeenCalledTimes(1)) to verify
reload was triggered, then restore the spy with reloadSpy.mockRestore();
reference the existing reloadSpy, window.location.reload, mockFetch, and
TodoForm to locate where to insert this assertion.
| it('完了状態の切り替えが動作する', async () => { | ||
| const user = userEvent.setup() | ||
| const mockFetch = vi.mocked(fetch) | ||
|
|
||
| mockFetch.mockResolvedValueOnce({ | ||
| ok: true, | ||
| json: async () => ({ ...mockTodo, completed: true }), | ||
| } as Response) | ||
|
|
||
| render( | ||
| <TodoItem | ||
| todo={mockTodo} | ||
| onUpdate={mockOnUpdate} | ||
| onDelete={mockOnDelete} | ||
| /> | ||
| ) | ||
|
|
||
| const checkbox = screen.getByRole('checkbox') | ||
| await user.click(checkbox) | ||
|
|
||
| await waitFor(() => { | ||
| expect(mockFetch).toHaveBeenCalledWith('/api/todos/1', { | ||
| method: 'PATCH', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| body: JSON.stringify({ | ||
| completed: true, | ||
| }), | ||
| }) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Missing assertion for onUpdate callback.
The test verifies the fetch call but should also assert that mockOnUpdate is called after a successful response.
Proposed fix
await waitFor(() => {
expect(mockFetch).toHaveBeenCalledWith('/api/todos/1', {
method: 'PATCH',
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify({
completed: true,
}),
})
})
+
+ await waitFor(() => {
+ expect(mockOnUpdate).toHaveBeenCalled()
+ })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('完了状態の切り替えが動作する', async () => { | |
| const user = userEvent.setup() | |
| const mockFetch = vi.mocked(fetch) | |
| mockFetch.mockResolvedValueOnce({ | |
| ok: true, | |
| json: async () => ({ ...mockTodo, completed: true }), | |
| } as Response) | |
| render( | |
| <TodoItem | |
| todo={mockTodo} | |
| onUpdate={mockOnUpdate} | |
| onDelete={mockOnDelete} | |
| /> | |
| ) | |
| const checkbox = screen.getByRole('checkbox') | |
| await user.click(checkbox) | |
| await waitFor(() => { | |
| expect(mockFetch).toHaveBeenCalledWith('/api/todos/1', { | |
| method: 'PATCH', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| }, | |
| body: JSON.stringify({ | |
| completed: true, | |
| }), | |
| }) | |
| }) | |
| }) | |
| it('完了状態の切り替えが動作する', async () => { | |
| const user = userEvent.setup() | |
| const mockFetch = vi.mocked(fetch) | |
| mockFetch.mockResolvedValueOnce({ | |
| ok: true, | |
| json: async () => ({ ...mockTodo, completed: true }), | |
| } as Response) | |
| render( | |
| <TodoItem | |
| todo={mockTodo} | |
| onUpdate={mockOnUpdate} | |
| onDelete={mockOnDelete} | |
| /> | |
| ) | |
| const checkbox = screen.getByRole('checkbox') | |
| await user.click(checkbox) | |
| await waitFor(() => { | |
| expect(mockFetch).toHaveBeenCalledWith('/api/todos/1', { | |
| method: 'PATCH', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| }, | |
| body: JSON.stringify({ | |
| completed: true, | |
| }), | |
| }) | |
| }) | |
| await waitFor(() => { | |
| expect(mockOnUpdate).toHaveBeenCalled() | |
| }) | |
| }) |
🤖 Prompt for AI Agents
In `@tests/components/TodoItem.test.tsx` around lines 43 - 74, Add an assertion
that the onUpdate callback is invoked after the successful PATCH response: after
the existing fetch assertion, await a waitFor (or use expect with async) to
assert mockOnUpdate was called with the updated todo object (e.g., the original
mockTodo with completed: true); reference the TodoItem component props and the
mockOnUpdate and mockFetch mocks to locate where to add this check.
| it('削除ボタンが動作する', async () => { | ||
| const user = userEvent.setup() | ||
| const mockFetch = vi.mocked(fetch) | ||
|
|
||
| mockFetch.mockResolvedValueOnce({ | ||
| ok: true, | ||
| json: async () => ({ message: 'TODOを削除しました' }), | ||
| } as Response) | ||
|
|
||
| render( | ||
| <TodoItem | ||
| todo={mockTodo} | ||
| onUpdate={mockOnUpdate} | ||
| onDelete={mockOnDelete} | ||
| /> | ||
| ) | ||
|
|
||
| const deleteButton = screen.getByTitle('削除') | ||
| await user.click(deleteButton) | ||
|
|
||
| await waitFor(() => { | ||
| expect(mockFetch).toHaveBeenCalledWith('/api/todos/1', { | ||
| method: 'DELETE', | ||
| }) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Missing assertion for onDelete callback.
Similar to the toggle test, this should verify that mockOnDelete is called after successful deletion.
Proposed fix
await waitFor(() => {
expect(mockFetch).toHaveBeenCalledWith('/api/todos/1', {
method: 'DELETE',
})
})
+
+ await waitFor(() => {
+ expect(mockOnDelete).toHaveBeenCalled()
+ })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('削除ボタンが動作する', async () => { | |
| const user = userEvent.setup() | |
| const mockFetch = vi.mocked(fetch) | |
| mockFetch.mockResolvedValueOnce({ | |
| ok: true, | |
| json: async () => ({ message: 'TODOを削除しました' }), | |
| } as Response) | |
| render( | |
| <TodoItem | |
| todo={mockTodo} | |
| onUpdate={mockOnUpdate} | |
| onDelete={mockOnDelete} | |
| /> | |
| ) | |
| const deleteButton = screen.getByTitle('削除') | |
| await user.click(deleteButton) | |
| await waitFor(() => { | |
| expect(mockFetch).toHaveBeenCalledWith('/api/todos/1', { | |
| method: 'DELETE', | |
| }) | |
| }) | |
| }) | |
| it('削除ボタンが動作する', async () => { | |
| const user = userEvent.setup() | |
| const mockFetch = vi.mocked(fetch) | |
| mockFetch.mockResolvedValueOnce({ | |
| ok: true, | |
| json: async () => ({ message: 'TODOを削除しました' }), | |
| } as Response) | |
| render( | |
| <TodoItem | |
| todo={mockTodo} | |
| onUpdate={mockOnUpdate} | |
| onDelete={mockOnDelete} | |
| /> | |
| ) | |
| const deleteButton = screen.getByTitle('削除') | |
| await user.click(deleteButton) | |
| await waitFor(() => { | |
| expect(mockFetch).toHaveBeenCalledWith('/api/todos/1', { | |
| method: 'DELETE', | |
| }) | |
| }) | |
| await waitFor(() => { | |
| expect(mockOnDelete).toHaveBeenCalled() | |
| }) | |
| }) |
🤖 Prompt for AI Agents
In `@tests/components/TodoItem.test.tsx` around lines 76 - 101, The test for
deletion in TodoItem is missing an assertion that the onDelete callback
(mockOnDelete) is invoked after a successful DELETE; update the test to assert
mockOnDelete was called (e.g., expect(mockOnDelete).toHaveBeenCalled() or
toHaveBeenCalledWith(mockTodo.id)) after the mockFetch resolves — place this
assertion after or inside the existing waitFor that checks fetch so it runs once
the deletion completes.
Summary by CodeRabbit
Release Notes
New Features
Tests
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.